Add backoff mechanism for 429, throw friendly message on REST API error and validate authorization during discovery mode #85
Conversation
…orization of the credentials passed in the config json file
| @@ -68,7 +68,7 @@ def ensure_credentials_are_valid(config): | |||
| XeroClient(config).filter("currencies") | |||
|
|
|||
| def discover(ctx): | |||
| ctx.refresh_credentials() | |||
| ctx.check_platform_access() | |||
There was a problem hiding this comment.
Code added w.r.to TDL-594 (Check authentication and authorization during discovery mode)
tap_xero/client.py
Outdated
| self.tenant_id = config['tenant_id'] | ||
|
|
||
|
|
||
| def check_platform_access(self, config, config_path, check_authentication=True): |
There was a problem hiding this comment.
Code added w.r.to TDL-594 (Check authentication and authorization during discovery mode)
| update_config_file(config, config_path) | ||
| self.access_token = resp["access_token"] | ||
| self.tenant_id = config['tenant_id'] | ||
| if resp.status_code != 200: |
There was a problem hiding this comment.
Code added w.r.to TDL-594 (Check authentication and authorization during discovery mode)
|
|
||
| BASE_URL = "https://api.xero.com/api.xro/2.0" | ||
|
|
||
|
|
||
| class XeroError(Exception): |
There was a problem hiding this comment.
Code added w.r.to TDL-648 (Permission Denied error should throw a user friendly message)
| api_rate_limit_message = ERROR_CODE_EXCEPTION_MAPPING[429]["message"] | ||
| message = "HTTP-error-code: 429, Error: {}. Please retry after {} seconds".format(api_rate_limit_message, resp_headers.get("Retry-After")) | ||
| # Handling status code 403 specially since response of API does not contain enough information | ||
| elif error_code in (403, 401): |
There was a problem hiding this comment.
Code added w.r.to TDL-648 (Permission Denied error should throw a user friendly message)
karanpanchal-crest
changed the title
| """ | ||
|
|
||
| @mock.patch('requests.Request', side_effect=mocked_badrequest_400_error) | ||
| def test_badrequest_400_error(self, mocked_session, mocked_badrequest_400_error): |
There was a problem hiding this comment.
Test cases to verify TDL-648 (Verifying that different error codes being handled gracefully)
|
|
||
|
|
||
| @mock.patch('requests.Request', side_effect=mocked_failed_429_request) | ||
| def test_too_many_requests_429_error(self, mocked_session, mocked_failed_429_request): |
There was a problem hiding this comment.
Test cases to verify TDL-593 (Verifying the backoff behavior with Error code 429)
|
|
||
| @mock.patch('requests.Session.send', side_effect=mocked_session) | ||
| class TestCheckPlatformAccessBehavior(unittest.TestCase): | ||
|
|
There was a problem hiding this comment.
Test cases to verify TDL-594 (Verifying the authentication and authorization behavior during discovery mode)
tap_xero/client.py
Outdated
| json.decoder.JSONDecodeError, | ||
| max_tries=3) | ||
| @backoff.on_exception(backoff.expo,json.decoder.JSONDecodeError,max_tries=3) | ||
| @backoff.on_exception(backoff.expo, XeroTooManyError, max_tries=3, factor=2) |
There was a problem hiding this comment.
The exceptions can be combined to form a single backoff statement as a tuple (https://github.com/litl/backoff#backoffon_exception)
ex:
@backoff.on_exception(backoff.expo, (XeroTooManyError, json.decoder.JSONDecodeError), max_tries=3, factor=2)
There was a problem hiding this comment.
@KAllan357 Incorporated the suggestion
Description of change
Since below JIRAs have interdependent code, I have added code for all these JIRAs in this single PR
Manual QA steps
Risks
Rollback steps